Skip to content

fix(custom-element): optimize slot retrieval to avoid duplicates #13961

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

fix(custom-element): optimize slot retrieval to avoid duplicates #13961

wants to merge 4 commits into from
+49 −4

Conversation

@edison1105
Copy link
Member

@edison1105 edison1105 commented Oct 4, 2025 •
edited by coderabbitai bot
Loading

close #13955

Summary by CodeRabbit

  • Bug Fixes

    • Improved slot aggregation for custom elements without shadow DOM: slots are now collected across all root contexts (including teleport targets) and deduplicated, preventing duplicate slot entries and ensuring more consistent slot content rendering in complex layouts.

  • Tests

    • Added a test validating teleport targets that are ancestors of a custom element host, confirming correct rendering boundaries and cleanup on unmount.

@coderabbitai
Copy link

coderabbitai bot commented Oct 4, 2025 •
edited
Loading

Walkthrough

Updates VueElement._getSlots to iterate all root nodes (host plus Teleport targets), collect slot elements into a Set to deduplicate across roots, and return the aggregated array, preventing duplicate slot handling when Teleport targets overlap with the custom element host.

Changes

Cohort / File(s) Summary
Custom element slot collection
packages/runtime-dom/src/apiCustomElement.ts		 Rewrote _getSlots to iterate over all root nodes (host + teleport targets), query slot elements per root, insert them into a Set to deduplicate, and return Array.from(set) instead of concatenating per-root arrays.
Tests: Teleport + custom element
packages/runtime-dom/__tests__/customElement.spec.ts		 Added test "teleport target is ancestor of custom element host" to verify Teleport behavior when the Teleport target is an ancestor of the custom element host, asserting expected DOM and successful unmount without slot render errors.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Browser
  participant CE as VueCustomElement
  participant Roots as Root Nodes (host + Teleport targets)
  participant Set as Slot Set

  Browser->>CE: mount custom element (shadowRoot: false)
  CE->>Roots: discover root nodes (host + teleport targets)
  loop for each root
    CE->>Roots: query slot elements
    Roots-->>Set: add slot elements (deduplicated)
  end
  CE->>CE: Array.from(Set) -> slots array
  CE->>Browser: render slots (deduplicated)
  note right of CE: prevents null insertBefore when teleport target is ancestor
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

I hopped through roots with twitching whiskers bright,
Gathered stray slots in a Set by moonlight.
No doubles now tumble, no insertBefore fright —
A tidy render, swift and light. 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title concisely and clearly summarizes the main change by indicating that slot retrieval in custom elements is optimized to avoid duplicates. It directly reflects the code modification, which switches to a Set-based approach to dedupe slot elements. The title follows conventional commit style and provides a clear, specific description of the update.
Linked Issues Check ✅ Passed The pull request addresses the linked issue by updating the slot retrieval logic to handle multiple roots including teleport targets and prevent null-node errors when the teleport target is an ancestor of the host, and it adds a new test validating this scenario. This change directly prevents the crash in _renderSlots described in issue #13955 and ensures slotted content is reliably retrieved without duplicates or null insertBefore operations. The test confirms the expected behavior when the teleport target is an ancestor, satisfying the issue requirements.
Out of Scope Changes Check ✅ Passed All code modifications are restricted to the custom element slot retrieval logic and its corresponding test suite, directly targeting the behavior described in the linked issue. There are no unrelated or extraneous changes in other modules or public API declarations. The pull request remains focused on fixing the duplicate slot retrieval and crash scenario without introducing out-of-scope updates.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch edison/fix/13955
📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b661cd3 and 72b5019.

📒 Files selected for processing (1)
  • packages/runtime-dom/src/apiCustomElement.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/runtime-dom/src/apiCustomElement.ts

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link

github-actions bot commented Oct 4, 2025 •
edited
Loading

Size Report

Bundles

File Size Gzip Brotli
runtime-dom.global.prod.js 102 kB (+42 B) 38.6 kB (+5 B) 34.7 kB (-16 B)
vue.global.prod.js 160 kB (+42 B) 58.7 kB (+7 B) 52.2 kB (-15 B)

Usages

Name Size Gzip Brotli
createApp (CAPI only) 46.7 kB 18.3 kB 16.7 kB
createApp 54.7 kB 21.3 kB 19.5 kB
createSSRApp 58.9 kB 23 kB 21 kB
defineCustomElement 60 kB (+42 B) 23 kB (+16 B) 21 kB (+4 B)
overall 68.8 kB 26.5 kB 24.2 kB

@pkg-pr-new
Copy link

pkg-pr-new bot commented Oct 4, 2025 •
edited
Loading

Open in StackBlitz

@vue/compiler-core

npm i https://pkg.pr.new/@vue/compiler-core@13961

@vue/compiler-dom

npm i https://pkg.pr.new/@vue/compiler-dom@13961

@vue/compiler-sfc

npm i https://pkg.pr.new/@vue/compiler-sfc@13961

@vue/compiler-ssr

npm i https://pkg.pr.new/@vue/compiler-ssr@13961

@vue/reactivity

npm i https://pkg.pr.new/@vue/reactivity@13961

@vue/runtime-core

npm i https://pkg.pr.new/@vue/runtime-core@13961

@vue/runtime-dom

npm i https://pkg.pr.new/@vue/runtime-dom@13961

@vue/server-renderer

npm i https://pkg.pr.new/@vue/server-renderer@13961

@vue/shared

npm i https://pkg.pr.new/@vue/shared@13961

vue

npm i https://pkg.pr.new/vue@13961

@vue/compat

npm i https://pkg.pr.new/@vue/compat@13961

commit: 0673542

@edison1105 edison1105 added 🔨 p3-minor-bug Priority 3: this fixes a bug, but is an edge case that only affects very specific usage. scope: custom elements labels Oct 4, 2025
@edison1105 edison1105 added the ready to merge The PR is ready to be merged. label Oct 4, 2025
@edison1105 edison1105 removed this from Next Minor Oct 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

🔨 p3-minor-bug Priority 3: this fixes a bug, but is an edge case that only affects very specific usage. ready to merge The PR is ready to be merged. scope: custom elements

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[custom-element] Crash in _renderSlots when Teleport target is ancestor of custom element host (shadowRoot: false)

2 participants

@edison1105